feat(vast): add Vast.ai SSH lease provider#680
Merged
Conversation
Contributor
|
Codex review: found issues before merge. Reviewed July 3, 2026, 4:48 AM ET / 08:48 UTC. Summary Reproducibility: not applicable. This PR adds a new provider rather than fixing a current-main bug. The relevant verification path is the live Vast smoke, and the PR body reports an authenticated exact-head create/use/destroy run. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land after maintainers accept the built-in Vast provider scope and cleanup contract, and resolve whether the release-owned changelog entry stays or moves to PR-body release-note context. Do we have a high-confidence way to reproduce the issue? Not applicable; this PR adds a new provider rather than fixing a current-main bug. The relevant verification path is the live Vast smoke, and the PR body reports an authenticated exact-head create/use/destroy run. Is this the best way to solve the issue? Unclear: the adapter shape matches existing direct SSH-lease provider patterns and the live proof now covers the key lifecycle, but first-party billable-provider acceptance and release-owned changelog handling need maintainer judgment. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 79c985cbf114. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
the
merge-risk: 🚨 security-boundary
Contributor
Author
|
@clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. |
steipete
force-pushed
the
vast-provider
branch
2 times, most recently
from
July 2, 2026 19:12
8f13285 to
19523ad
Compare
Add the first-party Vast provider configuration surface with env-only API-key sourcing, safe config output, provider flags, built-in registration, and offline placeholder backend contracts for later lifecycle plans. Add focused tests for Vast config precedence, credential destination provenance, provider discovery, flag registration, validation, and secret redaction.
Add the provider-local Vast.ai HTTP client, request and response models, payload builders, redacted API error handling, and compact ownership label helpers for the upcoming lifecycle implementation. Cover the client contract with fake HTTP tests for auth, redirects, offer search, instance creation and management, SSH-key methods, response decoding, redaction, and label ownership safety.
Add the Vast SSH-lease backend for acquire, resolve, list, release, touch, doctor, and cleanup using generated per-lease keys and strict cbx1 ownership labels. Cleanup is guarded by local claims plus provider labels, release destroys by default, and lifecycle tests use a fake Vast API/readiness path without live credentials.
Use fileURLToPath for the Cloudflare Workers mock alias so Vitest resolves the local mock correctly from repository paths that contain spaces.
Respect the generic SSH user override when Vast provider defaults are applied, so provider-specific defaults cannot replace an operator-supplied SSH user. Add regression coverage for both the Vast backend config and embedded direct SSH backend config.
Keep cleanup enabled for possible partial Vast warmup failures, but do not let a pre-acquire not-found stop response convert a known capacity blocker into cleanup_failed. Add regression coverage for the no-lease cleanup path.
Keep config show from replacing an explicitly supplied generic SSH user with the Vast provider default. This keeps the displayed effective config aligned with the backend defaulting path.
Report the release action captured on the lease target instead of the current config, so stopped or kept Vast instances are not reported as destroyed. Also route cleanup of owned-looking instances without matching local claims through the safe missing-claim refusal path.
Give local fake-SSH probe tests the same headroom as other terminal-bound checks so transient scheduling delays do not fail unrelated provider verification.
Preserve persisted Vast release and key metadata when normal resolve refreshes local claim endpoints, so stop/keep leases do not drift back to destroy. Also allow status-only resolution for owned instances that do not yet expose an SSH endpoint.
Detach provider-owned Vast SSH keys before destroying the instance so normal release cleanup follows the same effective ordering as rollback. Add an order assertion to prevent the detach call from becoming a suppressed post-destroy no-op.
Let an explicitly supplied Vast release action override the persisted claim label during release and reporting. This keeps safety overrides such as --vast-release-action keep from being ignored on leases acquired with the default destroy policy.
Give controller subprocess tests more non-race scheduling headroom after repeated local failures in terminal-bound process lifecycle checks. This keeps provider verification from failing on unrelated launch-gate timing.
Encode Vast search and create requests using the documented API shapes, restore stored SSH keys on resolve, and keep stopped lease claims so retained instances can be reconciled or destroyed later. Also widen a race-instrumented Apple VZ helper test timeout encountered during the final verification gate.
Drop unused Vast helper functions left behind after the pagination and release-lifecycle fixes. CI runs deadcode with test reachability, and these helpers were no longer referenced by the provider implementation or tests.
Reject query strings and fragments in the Vast API URL so configured provider endpoints cannot smuggle secret-bearing URL components or break path construction. Document the endpoint contract and cover query and fragment inputs in provider validation tests.
clawsweeper
Bot
added
proof: sufficient
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #363
200 {"instances":[]}responses.VISION.mdlifecycle boundaries and comprehensive Vast provider documentation. Changelog credit retained for @coygeek.Verification
Exact head:
bd051ada04060feb14bc482b7dd7d8ce2fc11a13.go vet ./...go test -race ./...node --test scripts/live-vast-smoke.test.js— 10 passedbash -n scripts/live-vast-smoke.shscripts/check-docs.sh— 51 command docs, 70 providers, 203 Markdown files, generated docs sitegit diff --checkAuthenticated Vast proof
Run against the exact head above with the guarded
$0.50/hourcap, one required GPU, destroy release policy, andnvidia/cuda:12.4.1-base-ubuntu22.04to avoid the development image's long pull:auth=ready control_plane=ready inventory=ready leases=043686387, leasecbx_9ad23b697061, RTX 5070 Tinvidia-smi -Lproved one GPUhttps://console.vast.ai/api/v0pre_owned=0 post_owned=0 cleanup=complete; authenticated doctor again reportedleases=0Failure-path canaries also proved rollback after rejected create payloads, live SSH-key response drift, a success-only mutation response, and an intentionally aborted slow image pull; every attempt returned to zero owned instances with no retained local claim or key material.